Skip to content

test: cover xarm room manipulation flow - #3761

Draft
mustafab0 wants to merge 1 commit into
mainfrom
test/xarm-room-sim-e2e
Draft

test: cover xarm room manipulation flow#3761
mustafab0 wants to merge 1 commit into
mainfrom
test/xarm-room-sim-e2e

Conversation

@mustafab0

@mustafab0 mustafab0 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Run .venv/bin/pytest -m mujoco -q dimos/e2e_tests/test_xarm_room_sim.py to test this PR.

@greptile-apps

greptile-apps Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds MuJoCo acceptance coverage for xArm room scanning, registration, and manipulation. The scene-truth check can reject a correctly registered object when its visible RGB-D point cloud is partial or asymmetric: it compares the visible-point centroid to the collision geometry origin with a 2 cm limit. Use compatible pose semantics or a visibility-aware comparison before merging.

Confidence Score: 4/5

Not safe to merge until the scene-truth assertion no longer treats a partial point-cloud centroid as the collision geometry origin.

A targeted executable harness ran the repository's centroid and matcher logic, showing that a balanced observation passes while a partial asymmetric observation of the same correctly registered object is rejected.

Files Needing Attention: dimos/e2e_tests/test_xarm_room_sim.py needs its point-cloud-to-scene-truth position assertion revised.

T-Rex T-Rex Logs

What T-Rex did

  • Submitted a proof for the posted P1 finding and linked it to the reviewer comment.
  • Validated the harness output by confirming the balanced point cloud was accepted at the collision origin.
  • Validated that a partial asymmetric point cloud was rejected despite sharing the collision origin.
  • For general contract validation, confirmed the complete observation at center [0.0, 0.0, 0.1] with zero origin error was accepted.
  • For the same validation, confirmed the partial observation at center [0.04, 0.0, 0.1] had x-axis error 0.040 m and was rejected by the PR assertion (>0.020 m).

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Point-cloud centroid is incorrectly required to equal collision geometry origin

    • Bug
      • _detected_objects reads the observed point-cloud centroid, while _scene_truth supplies a collision proxy origin. A partially visible asymmetric object can be correctly registered at the same origin yet have an observed centroid farther than the 2 cm threshold; the executed 8 cm-wide partial sample was rejected at 4 cm x error.
    • Cause
      • PointCloud2.center is the arithmetic mean of only observed points, not an object-pose or collision-geometry-origin estimate.
    • Fix
      • In dimos/e2e_tests/test_xarm_room_sim.py:162-200, do not compare pointcloud.center to collision geometry origins. Validate registration with an origin/pose estimate designed for that semantic, or compare geometry/point-cloud overlap with a visibility-aware tolerance.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "test: cover xarm room manipulation flow" | Re-trigger Greptile

Comment thread dimos/e2e_tests/test_xarm_room_sim.py
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4700 1 4699 39
View the top 1 failed test(s) by shortest run time
dimos.web.relay_bridge.test_relay_e2e::test_reset_burst_does_not_wedge_robot_leg
Stack Traces | 5.24s run time
robot = <dimos.web.relay_bridge.wt_client.RelayClient object at 0x7f1426d2aff0>
viewer = <dimos.web.relay_bridge.wt_client.RelayClient object at 0x7f1426d56480>

    async def test_reset_burst_does_not_wedge_robot_leg(
        robot: RelayClient, viewer: RelayClient
    ) -> None:
        """Resets racing stream acceptance must not kill the relay's robot data path.
    
        A stream reset before the relay has read its WebTransport preamble errors
        Deno's wt.incomingBidirectionalStreams permanently (rejected pull), which
        used to silently end the robot stream loop. Bursting resets in the same
        event-loop turn as the sends makes that race near-certain.
        """
        await attach(robot, viewer, ["cam"])
        for rnd in range(5):
            # The accept glue cannot have read all 50 preambles before the
            # resets land, so some streams are reset pre-acceptance.
            ids = [robot.send_frame("cam", b"\xcd" * (16 * 1024), delivery="latest") for _ in range(50)]
            for stream_id in ids:
                robot._session.reset_if_in_flight(stream_id)
            marker = f"alive-{rnd}".encode()
            robot.send_frame("cam", marker, delivery="latest")
    
            frames = await collect_until(
                viewer,
                lambda fs, marker=marker: any(bytes(f.payload) == marker for f in fs),
                timeout=5.0,
            )
>           assert any(bytes(f.payload) == marker for f in frames), (
                f"robot data path wedged in round {rnd}"
            )
E           AssertionError: robot data path wedged in round 2
E           assert False
E            +  where False = any(<generator object test_reset_burst_does_not_wedge_robot_leg.<locals>.<genexpr> at 0x7f1426d61b60>)

frames     = [DataFrame(header=FrameHeader(ch='cam', seq=102, ts=1788054290.7532084, delivery='latest', meta=None), payload=b'\xcd\...\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd')]
ids        = [412, 416, 420, 424, 428, 432, ...]
marker     = b'alive-2'
rnd        = 2
robot      = <dimos.web.relay_bridge.wt_client.RelayClient object at 0x7f1426d2aff0>
stream_id  = 608
viewer     = <dimos.web.relay_bridge.wt_client.RelayClient object at 0x7f1426d56480>

.../web/relay_bridge/test_relay_e2e.py:255: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 29, 2026
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 0b84441 to 500c72f Compare August 29, 2026 19:06
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 29, 2026
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 500c72f to 7049e48 Compare August 29, 2026 19:45
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 7049e48 to c6eb07b Compare August 29, 2026 19:50
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch 2 times, most recently from 345627f to b2b246f Compare August 29, 2026 20:12
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from b2b246f to 6c954ad Compare August 29, 2026 20:18
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 29, 2026
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 6c954ad to 428299e Compare August 29, 2026 22:50
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 29, 2026
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 428299e to ffeea5c Compare August 29, 2026 22:53
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from ffeea5c to 9fc9db2 Compare August 30, 2026 01:05
@mustafab0
mustafab0 marked this pull request as draft August 30, 2026 01:17
Base automatically changed from feat/xarm-room-sim to main August 30, 2026 01:39
@mustafab0
mustafab0 force-pushed the test/xarm-room-sim-e2e branch from 9fc9db2 to 491aafa Compare August 30, 2026 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant